Skip to content

refactor(coding-agent): wake RLM quiescence on activity changes instead of polling - #1832

Closed
snimu wants to merge 180 commits into
mainfrom
snimu/rlm-activity-change-waiter
Closed

snimu wants to merge 180 commits into
mainfrom
snimu/rlm-activity-change-waiter

Conversation

@snimu

@snimu snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

What was wrong

waitForRlmQuiescence — the strong idle barrier gating goal continuations and daemon idle for sessions with subagent work — detected settlement by re-checking on a zero-delay setTimeout(0) loop: busy-spinning the event loop instead of being told when activity changes (audit: timeouts.md finding 7).

The fix

The poll is replaced by an abort-aware activity-change waiter that reuses the existing session-input checkpoint waiter set (no second notification registry). Every component of the quiescence predicate now notifies on its clear path — bash, refine, retry, compaction, branch summaries, terminal actions, and (added in review) the post-compaction continuation settlement, which was the one clear path without a notify and could have hung the barrier permanently in a narrow registration window. Spurious wakes are safe in both directions: all consumers of the shared set re-check their predicates in loops. +31/−9.

How it's verified

Reviewer performed an exhaustive missed-wake audit of every isSessionActive component with per-clear-site accounting, verified bidirectional spurious-wake safety across the shared waiter set, confirmed no hot-path notifies (all fire once per operation, never per-token), and validated the goal-continuation-quiescence suite (#1610) is untouched and green (7/7). Recursion suite 116/116; full CI-style failing set matches stack base. Two-model implement/review loop, approved on second pass.

Stacked on #1751 (test the whole stack at the leaf; merge base-first).

Note: intentionally no Linear ticket for this cleanup stack, so that check stays red.


Note

Medium Risk
Changes touch session continuation, RLM quiescence, daemon messaging, and RPC timeout defaults—areas where subtle hangs or duplicate messages are possible if lifecycle notifications or peer listing regress.

Overview
This PR replaces several timeout-and-poll patterns with lifecycle notifications and tightens daemon/TUI/RPC consistency around queues, peers, and RLM child state.

Session runtime (agent-session) — Post-compaction continuation no longer uses a 100ms timer; it loops on real readiness (agent idle, retry, refine, queued-work pauses, compaction) and can continueAfterSessionInput when session-owned work finishes first. RLM quiescence waits on _waitForSessionActivityChange instead of setTimeout(0) while session work is active; bash/refine/retry/compaction paths notify checkpoint waiters. RLM children are retained with run metadata so reattach snapshots include queued/running children, activity, previews, and duration; daemon child listing delegates to getRlmChildSnapshots().

Daemon — Cross-worker agent rosters are pulled on demand via new list_agent_peers (schema revision 23); worker_sync_agent_peers and the remoteAgentPeers cache are removed. Remote send_message connects first, sends once, and does not resend on post-send failure. Shutdown waits on inFlightBash instead of polling isBashRunning. Supervised workers apply renames through applyStateSessionName so ledger updates match supervisor approval. Legacy RLM registry parsing is shared from rlm-ledger.

Interactive TUI — Queued messages and scoped heartbeats are derived from connectionState (no mirrored queue); queue edit/move uses server mutations with refreshAt instead of optimistic local patches. New-chat hints use messageCount and streaming state.

RPC client — Removes fixed per-command timeouts and the startup 100ms delay; transport failures fail all pending requests/waiters. waitForIdle / collectEvents no longer default to 60s unless callers pass a timeout.

Smaller removals/fixes — Unused getOverflowPatterns API; test-only config cache reset and daemon lookup override; empty selector auto-cancel timers; NODE_ENV=test implicit telemetry off; kernel dispose cancels in-flight execution before final snapshot instead of a separate dispose timeout cap.

Reviewed by Cursor Bugbot for commit bffbda5. Bugbot is set up for automated code reviews on this repo. Configure here.

Linear ticket: ENG-5667
(ticket linked above)

Note

Wake RLM quiescence on activity changes instead of polling and harden RpcClient failure handling

  • Replaces zero-delay polling in AgentSession._waitForSessionActivityChange with an explicit waiter that resolves on session activity checkpoint notifications, used by strong RLM quiescence yielding
  • Reworks RpcClient to remove per-request and blanket timeouts: start awaits child spawn readiness, send relies on response or centralized failPendingOperations, and waitForIdle/collectEvents/promptAndWait support optional timeouts with transport-error abort
  • Refactors InteractiveMode to derive heartbeats and queue state on-demand from connectionState instead of maintaining mirrored heartbeats, connectionQueue, and sessionHasMessages fields; queue reorders and edits now go through server mutations with post-mutation selection reconciliation via refreshQueueSelectionAt
  • Switches daemon peer discovery from supervisor-pushed syncAgentPeers to pull-based list_agent_peers (schema revision 23); adds inFlightBash tracking so daemon shutdown awaits bash completion without polling
  • Risk: HeartbeatManagerComponent constructor signature changed — consumers must pass getHeartbeats in options instead of an initial array; RpcClient.send no longer accepts a timeoutMs parameter; clearApiKeyCache and getOverflowPatterns exports removed from model-registry and overflow modules respectively

Macroscope summarized bffbda5.


Supersedes #1753 (recreated as a plain PR against main; GitHub's stack lock prevented retargeting the stacked PR).

snimu added 30 commits August 24, 2026 11:17

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium

const identity = this.processIdentity(worker.descriptor.pid, worker.descriptor.processStartId);

A supervisor-only restart leaves pre-revision-23 workers with stale remoteAgentPeers, so they can continue routing to reclaimed peers or miss replacements until those workers restart. Removing the post-reclaim syncAgentPeers call means these workers neither support list_agent_peers nor receive the legacy worker_sync_agent_peers update; preserve a compatibility synchronization path for older workers.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/daemon/daemon-supervisor.ts around line 2364:

A supervisor-only `restart` leaves pre-revision-23 workers with stale `remoteAgentPeers`, so they can continue routing to reclaimed peers or miss replacements until those workers restart. Removing the post-reclaim `syncAgentPeers` call means these workers neither support `list_agent_peers` nor receive the legacy `worker_sync_agent_peers` update; preserve a compatibility synchronization path for older workers.

Evidence trail:
packages/coding-agent/src/modes/daemon/daemon-supervisor.ts:697-742, 2337-2384, 2655-2674, 2707-2763, 3420-3425 at REVIEWED_COMMIT; packages/coding-agent/src/modes/daemon/daemon-mode.ts:5309-5335 at REVIEWED_COMMIT; packages/coding-agent/src/modes/daemon/daemon-protocol.ts:66-71, 716-722 at REVIEWED_COMMIT; git diff MERGE_BASE..REVIEWED_COMMIT -- packages/coding-agent/src/modes/daemon/daemon-supervisor.ts; commit 6e33a6ed0846c54fcb7cec63a785a90db95931de

this.connectionQueue = {
steering: [...queue.steering],
followUp: [...queue.followUp],
private getConnectionQueue(): AgentConnectionQueueState {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium interactive/interactive-mode.ts:2509

getConnectionQueue() can return a stale queue after rebindCurrentSession(): the initial getState() snapshot is fetched before subscribeToAgent(), so a queue change from another client in that gap is missed and queued previews/editing use outdated entries until a later event or resync. Restore a post-subscription queue refresh, or otherwise close this snapshot/subscription race.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/interactive/interactive-mode.ts around line 2509:

`getConnectionQueue()` can return a stale queue after `rebindCurrentSession()`: the initial `getState()` snapshot is fetched before `subscribeToAgent()`, so a queue change from another client in that gap is missed and queued previews/editing use outdated entries until a later event or resync. Restore a post-subscription queue refresh, or otherwise close this snapshot/subscription race.

Evidence trail:
packages/coding-agent/src/modes/interactive/interactive-mode.ts:2509-2514, 2516-2529, 2784-2802, 2657-2660, 5073-5085 at 4e4aeb0ac2d985d301903bc37920c9fe2ee74c7a
git diff MERGE_BASE..REVIEWED_COMMIT -- packages/coding-agent/src/modes/interactive/interactive-mode.ts
packages/coding-agent/src/modes/agent-connection/daemon-agent-connection.ts:382-386, 1612-1622, 2130-2139 at 4e4aeb0ac2d985d301903bc37920c9fe2ee74c7a

/** Best-effort final snapshot before a graceful dispose, bounded by a timeout. */
private async flushSnapshotForDispose(): Promise<void> {
if (!this.options.snapshot || !this.isRunning) return;
const pendingExecutions = this.executionQueue;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High kernel/repl-manager.ts:1104

dispose() and shutdown({ snapshot: true }) can wait indefinitely instead of tearing down the kernel when a new non-terminating execute() is queued after pendingExecutions is captured. That request is placed ahead of captureSnapshot(), and executionTimeoutMs is not armed until enqueueRequest() finishes waiting for the prior queue, so the snapshot timeout never starts. Prevent new executions from being enqueued while the final flush is being reserved, or otherwise reserve the snapshot queue position before waiting.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/core/kernel/repl-manager.ts around line 1104:

`dispose()` and `shutdown({ snapshot: true })` can wait indefinitely instead of tearing down the kernel when a new non-terminating `execute()` is queued after `pendingExecutions` is captured. That request is placed ahead of `captureSnapshot()`, and `executionTimeoutMs` is not armed until `enqueueRequest()` finishes waiting for the prior queue, so the snapshot timeout never starts. Prevent new executions from being enqueued while the final flush is being reserved, or otherwise reserve the snapshot queue position before waiting.

Evidence trail:
packages/coding-agent/src/core/kernel/repl-manager.ts:454-496, 1002-1020, 1102-1117, 1119-1146, 897-961 (reviewed commit 4e4aeb0ac2d985d301903bc37920c9fe2ee74c7a)

if (sessionGeneration !== this.sessionEventGeneration) return;
await this.sessionEventQueue;
if (sessionGeneration !== this.sessionEventGeneration) return;
this.refreshQueueSelectionAt(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium interactive/interactive-mode.ts:7023

After a successful reorder, moveQueueSelection can validate against the pre-move queue and reset the selection, restoring the draft even though the daemon moved the message. Awaiting sessionEventQueue does not guarantee that a backpressured client has processed the session_action_update; the later catch-up snapshot arrives after pendingQueueMove is cleared and validates the old index again. Reconcile the authoritative queue (or retain the expected post-move position until the corresponding update is applied) before resetting the selection.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/coding-agent/src/modes/interactive/interactive-mode.ts around line 7023:

After a successful reorder, `moveQueueSelection` can validate against the pre-move queue and reset the selection, restoring the draft even though the daemon moved the message. Awaiting `sessionEventQueue` does not guarantee that a backpressured client has processed the `session_action_update`; the later catch-up snapshot arrives after `pendingQueueMove` is cleared and validates the old index again. Reconcile the authoritative queue (or retain the expected post-move position until the corresponding update is applied) before resetting the selection.

Evidence trail:
packages/coding-agent/src/modes/interactive/interactive-mode.ts:2624-2629, 7002-7033; packages/coding-agent/src/modes/interactive/queue-selection.ts:63-83; packages/coding-agent/src/core/agent-session.ts:6395-6401; packages/coding-agent/src/modes/daemon/daemon-mode.ts:3558-3563, 6557-6604, 6770-6881; reviewed commit 4e4aeb0ac2d985d301903bc37920c9fe2ee74c7a

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bffbda5. Configure here.

clearTimeout(timeout);
resolve();
});
child.kill("SIGTERM");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RPC stop can hang forever

High Severity

RpcClient.stop() now waits only for the child close event. The one-second timer sends SIGKILL but never finishes the wait, so if close was already emitted or never arrives after kill, stop() hangs and later start() can keep seeing a live client.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit bffbda5. Configure here.

@snimu

snimu commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Superseded: the chained stack was restructured into independent PRs (byte-identical combined tree). A fresh standalone PR for this change follows on the same branch name.

@snimu snimu closed this Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant